fix(frontend): clear stale streamingSessionId on session reopen - #1257
Open
iagop03 wants to merge 1 commit into
Open
fix(frontend): clear stale streamingSessionId on session reopen#1257iagop03 wants to merge 1 commit into
iagop03 wants to merge 1 commit into
Conversation
The sidebar's "thinking" spinner for a backgrounded session (streamingSessionId) never cleared once its turn actually finished, because: - switchSession() deliberately preserves streamingSessionId across a navigation so the sidebar can keep showing progress for a session you've left, but the same switch effect also disconnects that session's SSE stream (doDisconnect()), so the session_completed event that would normally clear the marker (via clearStreamingSession) never arrives. - Reopening the session doesn't help either: /events?replay=active only replays buffered events for a still-running attempt, so there's nothing to reconcile against for one that already completed. loadSessionMessages() already fetches the session's full committed history on every open/reopen. If the session being loaded is the one currently marked as streaming and its newest stored message is the assistant's reply, that turn is done -- release the stale marker using data already in hand, no extra request needed. Closes HKUDS#1256 Signed-off-by: Iago Pueyo <iago.pueyo99@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Closes #1256.
streamingSessionId(the store field the sidebar spinner reads) is deliberately preserved when you switch sessions, so it can keep showing progress for a session you've left. But the same session-switch effect also tears down that session's SSE connection (doDisconnect()), so thesession_completedevent that would normally clear the marker never arrives. Reopening the session later doesn't help either —/events?replay=activeonly replays buffered events for a still-runningattempt, so there's nothing to reconcile against once it has already completed. The one function that does this reconciliation (syncCompletedAttempt) is only ever invoked right after the local client sends a message, never on session (re)open.Net effect: leave a streaming session, and its sidebar spinner is wrong forever, until a full page reload.
Changes
frontend/src/pages/Agent.tsx: inloadSessionMessages(already called on every session open/reopen, and already fetching that session's full committed history), if the session being loaded is the one marked asstreamingSessionIdand its newest stored message is already the assistant's reply, clear the stale marker viaclearStreamingSession. No extra network request — reuses data already fetched for this call.frontend/src/pages/__tests__/AgentBackgroundStreamCompletion.test.tsx: new regression test covering both the fix (stale marker clears once the reply is committed) and the negative case (marker is left alone when the background session genuinely has no reply yet, i.e. still running).Test Plan
npx vitest run src/pages/__tests__/AgentBackgroundStreamCompletion.test.tsx— 2/2 passing.npx vitest run --pool=threads— 596/598 passing; the 2 failures (options.test.tsivLabeldecimal-separator formatting,tearsheet.test.tsUTC-5 timezone-offset assumption) are pre-existing and locale/timezone-environment-dependent, unrelated to this change (reproduce identically onmainwithout this diff).npx tsc -b— clean.Checklist
src/agent/,src/session/,src/providers/)